Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tool to auto-generate solidity documentation for project. #136

Closed

Conversation

derekpierre
Copy link
Member

@derekpierre derekpierre commented Jan 24, 2023

Vict0r asked me to help him out with the documentation work.

Here is a first pass at using solidity-docgen from Open Zeppelin - https://github.com/OpenZeppelin/solidity-docgen.

Happy to make edits for minor changes. If major changes are needed however, you can either merge and build on this change with subsequent PRs, OR feel free to cherry-pick the commit and make any necessary adjustments as needed in a separate PR.

See docs/contracts for the auto-generated output - https://github.com/derekpierre/solidity-contracts/tree/automated-documentation/docs/contracts

@vzotova
Copy link
Contributor

vzotova commented Jan 24, 2023

For my state I'd not include in generated docs nothing more than public and external functions/variables (and events).

@derekpierre
Copy link
Member Author

derekpierre commented Jan 25, 2023

For my state I'd not include in generated docs nothing more than public and external functions/variables (and events).

Great point; pushed c103693.

Admittedly, I'm not trying to get this perfect, but rather to provide the devs with something quick as a base to build on. I'm sure the theme can be refined further.

@derekpierre
Copy link
Member Author

Any thoughts on getting this merged? Not sure what the GH action failures are about:

error Command failed.
Exit code: 128
Command: git
Arguments: ls-remote --tags --heads ssh://[email protected]/web3-js/scrypt-shim.git
Directory: /home/runner/work/solidity-contracts/solidity-contracts
Output:
[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

@pdyraga
Copy link
Member

pdyraga commented Mar 7, 2023

I would consider replacing/extending this PR to use the mechanism @michalinacienciala is working on in keep-network/tbtc-v2#557. The real benefit from generating contract docs is when we can host them. Here is Michalina's work-in-progress documentation generated for tBTC v2: https://api-docs.threshold.network/

@mswilkison
Copy link
Contributor

I would consider replacing/extending this PR to use the mechanism @michalinacienciala is working on in keep-network/tbtc-v2#557. The real benefit from generating contract docs is when we can host them. Here is Michalina's work-in-progress documentation generated for tBTC v2: https://api-docs.threshold.network/

This is very timely re: a discussion @mhluongo and I recently had about threshold docs. Is https://api-docs.threshold.network/ a placeholder WIP (with the idea that api docs will eventually live on docs.threshold.network) or is the goal to have api docs live separately?

@mswilkison
Copy link
Contributor

There is an App Development tab where this could live
image

@derekpierre
Copy link
Member Author

derekpierre commented Mar 8, 2023

Is https://api-docs.threshold.network/ a placeholder WIP (with the idea that api docs will eventually live on docs.threshold.network) or is the goal to have api docs live separately?

I'm also interested in the thought process here. Any API docs would have to include subpages for various applications across Threshold and not just tBTC.

We already have hosting via gitbook (docs.threshold.network) that can be leveraged. Depending on how the md files are produced by the various repositories, gitbook (used for docs.threshold.network) can import sub-pages from external sources - https://docs.gitbook.com/getting-started/import.

Here is one such import I played around with - I imported directly from api-docs.threshold.network:
Screenshot 2023-03-08 at 11 51 19 AM

Also here is an import from this PR for the page https://github.com/derekpierre/solidity-contracts/blob/automated-documentation/docs/contracts/Checkpoints.md:
Screenshot 2023-03-08 at 11 53 51 AM

I don't believe importing sub-pages allows for automated updates when the files get updated. Perhaps it needs to be re-imported after external changes 🤷 . Alternatively, I think there are some special things you can do with files in gitbook (done via direct commits to gitbook and probably not via the UI), to directly link to external sources for the API docs files so that the data can possibly change eg. github repos. Maybe something like this - https://stackoverflow.com/questions/42248880/how-to-import-a-markdown-file-from-github-into-local-gitbook - I have not tried this before, but something that can be experimented with.

Absolute worst case, we could link out to a docs folder in github with md files (but feels like we can have something reasonable instead of this).

@pdyraga
Copy link
Member

pdyraga commented Mar 9, 2023

I'm also interested in the thought process here. Any API docs would have to include subpages for various applications across Threshold and not just tBTC.

Agreed. The current state is just the first step. The goal is to have Solidity API from all repositories plus some other APIs, like the Typescript library for tBTC.

The goal of the work @michalinacienciala is doing (see keep-network/tbtc-v2#553) was to generate documentation automatically as a part of the CI process and publish it on a website. Worth noting, we had to add some intermediate steps to the process (see keep-network/tbtc-v2#557) to ensure the end result is rendered nicely in all cases. If we can import the generated files into GitBook and do not have a separate bucket for API documentation, this would be ideal. The bucket upload is just a small step at the very end that can be easily replaced.

I think we should aim at having it generated into a CI and happening automatically. Otherwise, the documentation will quickly get out of sync.

I am looking at GitBook import and I do not see a way to automate HTML imports. One way we could try is to have a separate repository for HTML documentation and have an automated process to push to that repository after every main merge. This would be an aggregator for HTML documentation Then, we'd sync GitBook with that repo. This is only replacing step 6 from keep-network/tbtc-v2#557. If importing *.md/ *.adoc files makes more sense, then we will also drop step 5 but I think it's not a big deal given all intermediate processing we had to do is done earlier (@michalinacienciala to confirm).

@michalinacienciala
Copy link
Contributor

michalinacienciala commented Mar 9, 2023

If I understand correctly how GitBook works, it offers a two-way synchronization between itself and a GitHub repository (in our case https://github.com/threshold-network/threshold/docs). This means that any changes applied to main in that repository under docs are automatically reflected in the GitBook content.
If that's a case, maybe we could do the following?
In each our project that deploys contracts we could configure a job (modified version of what I made in keep-network/tbtc-v2#557) that generates the Markdown file for the project (for example, for tbtc-v2, this could be tbtc-v2-solidity-api.md). The job would be configured to push the file (on release) to the threshold-network/threshold repo, under docs/app-development/api-docs/solidity-api/. For pushing the files to that repo, we could use for example this action: https://github.com/marketplace/actions/push-a-file-to-another-repository (the user associated with the API_TOKEN_GITHUB secret would have to have privileges to push directly to main).
We would probably also have to manually change the https://github.com/threshold-network/threshold/blob/main/docs/SUMMARY.md, to add the links to the generated files. But this would have to be done only once per supported project, so it's not particularly inconvenient.
Once we integrate more projects the structure could look like this:

threshold-network/threshold/docs/app-development/api-docs/
  solidity-api/
    threshold-solidity-api.md
    random-solidity-api.md
    ecdsa-solidity-api.md
    tbtc-v2-solidity-api.md
  typescript-api/
    abc-typescript-api.md
    ...

Some potential downsides of this solution:

  • I don't think we would be able to easily modify the table of contents to list all the functions in the individual files (as we do this when hosting the HTML files outside GitBook - see left menu on https://api-docs.threshold.network/). This would make the documents a bit harder to navigate (as some of them will have a lot of data), but it's something that I think we can live with...
  • Making changes to the files such as the ones discussed in Add tool to auto-generate solidity documentation for project. #136 (comment) would be hard to achieve in the automated way (although probably not impossible - maybe there is some complicated sed command that would allow us to get rid of what we don't need or other way to parse the file and remove unwanted bits - but it may not be worth our time).

What do you folks think about this? Could we do this the way I proposed? Any downsides I missed? Any solutions to the downsides I listed?

@michalinacienciala
Copy link
Contributor

One way we could try is to have a separate repository for HTML documentation and have an automated process to push to that repository after every main merge.

@pdyraga , so what I propose above is a bit similar to what you wrote, but instead of pushing files to a separate repository, we would push directly to the GitBook's repo. And I think we should push the docs not on pushes to main in the parent projects (like tbtc-v2/solidity), but on releases in those projects - because our API documentation should reflect the contracts that are deployed on Mainnet, not those that haven't been yet deployed (correct me if that's an incorrect assumption). And contracts on main may differ from those deployed on Mainnet if we're actively developing new functions/contracts etc.

@michalinacienciala
Copy link
Contributor

michalinacienciala commented Mar 9, 2023

Any thoughts on getting this merged? Not sure what the GH action failures are about:

error Command failed.
Exit code: 128
Command: git
Arguments: ls-remote --tags --heads ssh://[email protected]/web3-js/scrypt-shim.git
Directory: /home/runner/work/solidity-contracts/solidity-contracts
Output:
[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

I saw similar problems before in workflows triggered by changes in the forked repos. This was usualy related to the fact that

In order to protect public repositories for malicious users we [GitHub] run all pull request workflows raised from repository forks with a read-only token and no access to secrets.

I'm not sure if this practice is at fault this time, but I think there's a high chance the problem is related to the fact that the PR was crated from the fork.

@derekpierre
Copy link
Member Author

derekpierre commented Mar 9, 2023

If I understand correctly how GitBook works, it offers a two-way synchronization between itself and a GitHub repository (in our case https://github.com/threshold-network/threshold/docs). This means that any changes applied to main in that repository under docs are automatically reflected in the GitBook content.

@michalinacienciala Correct, and that is the current setup.

We would probably also have to manually change the https://github.com/threshold-network/threshold/blob/main/docs/SUMMARY.md, to add the links to the generated files. But this would have to be done only once per supported project, so it's not particularly inconvenient.

Potentially not just once, if additional files get generated. One possible way around this could be to always include a index.md in the respective repo, and have that link to the various files. That way the gitbook docs just link to the index file, and everything is shown on the index page is linked to relative from there.

And contracts on main may differ from those deployed on Mainnet if we're actively developing new functions/contracts etc.

Good question. This would be a argument in favor of a CI workflow for releases and pushing to the gitbook repos, unless release practices are potentially modified.

I guess I'm unfamiliar with the release process for other Keep repos. We, NuCypher, use the git flow model (https://nvie.com/posts/a-successful-git-branching-model/) where whatever is on main is the latest release, and active development occurs on the development branch. Once development has gotten to a reasonable point for a release, development is merged into main, and tagging for a release.

If the decision is to push files from a separate repos to the Gitbook repos (threshold-network/threshold) then these separate docs should probably have their own designated folders to prevent conflicts i.e. tbtcv2 has it's own folder etc. The drawback here I guess is that each repos must have such a CI workflow, which isn't some terrible thing.


Copying my musing on Discord here about a potential alternative:

Our singular docs GitBook space is currently backed by/synced to the threshold-network/threshold repos, and has one space "Threshold Docs" under the "Threshold" organization.

AFAIK each GitBook space (https://docs.gitbook.com/getting-started/content-structure/what-is-a-space) can only be linked to one GitHub repos. In terms of keeping various docs synced based on content produced in various GitHub repos, I think one possibility is to use a different GitBook space for each external repos containing docs. I believe then each GitBook space can be configured to be synced to a different GitHub repos docs - https://docs.gitbook.com/getting-started/git-sync/enabling-github-sync. Of course the various repos would need to be set up to allow GitBook to pull the docs appropriately.

That way changes to docs can continue to occur in these external GitHub repos, and then the corresponding GitBook space gets automatically updated. You could setup the sync as one-directional (disable live-edits) i.e. docs changes for those spaces must occur via the external git repos and not via GitBook to keep things simple especially for API type of docs.

You can link from one GitBook space to another, so the general Threshold Docs GitBook space can link to the other GitBook spaces (API docs or otherwise). All spaces would be under the existing singular Threshold organization in GitBook.

So potentially something like this in Gitbook:

[Threhold Org]:
      <Threshold Docs Space>
             - What is Threshold Network?
             - ...
             - Application Development
                 ...
                 - API Docs
                     - tBTCv2 API Docs
                              {link to TBTCv2 API Docs Space}
                     ...
                     - Threshold Access Control API Docs
                              {link to Threshold Access Control API Docs Space}
     <tBTCv2 API Docs Space> -> synced to repos docs folder <...>
     <Threshold Access Control API Docs Space> -> synced to repos docs folder <...>

The benefits are:

  • Github repos can maintain their own process for producing docs and they control the structure of their docs, and gitbook will simply sync with it.
  • No additional CI work to push changes and always updated in real-time.
  • Could possible prevent live edits i.e. changes from GitBook side for documentation generated from external repos (since the corresponding space could prevent live edits)

The drawbacks are:

  • Using more than one space - but not a huge deal
  • Initial work for github repos to sync appropriately with gitbook - probably need gitbook.yaml file.
  • Doesn't solve the issue of releases vs what's on main for Keep's release process - although perhaps gitbook can sync to a tag, so to get the latest release, just update gitbook space to use the relevant release tag for the repos. I'm assuming here that gitbook can point to a tag instead of a branch... 🤷

Caveat: not claiming to be well-versed in Gitbook, just what I've gathered from reading


Whatever method we end up deciding on, we should think about automating as much as possible and the ease of maintenance cost by the various teams.

@derekpierre
Copy link
Member Author

I'm not sure if this practice is at fault this time, but I think there's a high chance the problem is related to the fact that the PR was crated from the fork.

👍 Thanks for clarifying...this was a confusing one.

@michalinacienciala
Copy link
Contributor

michalinacienciala commented Mar 10, 2023

We would probably also have to manually change the https://github.com/threshold-network/threshold/blob/main/docs/SUMMARY.md, to add the links to the generated files. But this would have to be done only once per supported project, so it's not particularly inconvenient.

Potentially not just once, if additional files get generated. One possible way around this could be to always include a index.md in the respective repo, and have that link to the various files. That way the gitbook docs just link to the index file, and everything is shown on the index page is linked to relative from there.

I think both solutions require similar amount of work in case more files will be generated. You either have to modify SUMMARY.md in the threshold-network/threshold repo or index.md in the respective repo.


I guess I'm unfamiliar with the release process for other Keep repos. We, NuCypher, use the git flow model (https://nvie.com/posts/a-successful-git-branching-model/) where whatever is on main is the latest release, and active development occurs on the development branch. Once development has gotten to a reasonable point for a release, development is merged into main, and tagging for a release.

Oh, so it looks that Nu's process is different than Keep's. In Keep we merge the feature branches to main (based on which we publish NPM packages for development or testnet, versioned for example 1.0.1-dev.0, 1.0.1-goerli.5) and once we're ready for a release, we create a Release, tag it (for example solidity/v1.0.1) and publish an NPM package versioned without suffixes (for example 1.0.1).

This would be a argument in favor of a CI workflow for releases and pushing to the gitbook repos, unless release practices are potentially modified.
and

and

Doesn't solve the issue of releases vs what's on main for Keep's release process - although perhaps gitbook can sync to a tag, so to get the latest release, just update gitbook space to use the relevant release tag for the repos. I'm assuming here that gitbook can point to a tag instead of a branch... 🤷

I don't think Keep plans to modify the release practices anytime soon, the current approach seems to work fine for us. But anyway, I'm not sure if that really is an argument in favor of using CI workflow and pushing to the GitBook repo. We could still use the approach you suggested (with storing the docs in the respective repos that would be synched with individual GitBook spaces). This could be done in one of the following ways:
A) We create a dedicated path in the project's repo for storing documentation (for example ./solidity/api-docs) which will be existing in a dedicated branch of the project (for example tbtc-v2-api-docs). We synch that path & branch with a dedicated GitBook space. Then we create a workflow generating .md docs that gets triggered on the release event. It pushes the generated artifact(s) to that path & branch, updating the GitBook space content.
B) We create a dedicated path in the project's repo for storing documentation (for example ./solidity/api-docs) which will be existing on main. We synch that path & branch with with a dedicated GitBook space. Then we create a workflow generating .md docs that gets triggered on the release event. It pushes the generated artifact(s) to that path & branch, updating the GitBook space content. I'm not a fan of this second solution though, because we may often have a contradicting info in the .sol files and in the .md files on the main branch.
No matter if we would go with one of the above processes, or if we would go with pushing the generated file(s) to the threshold-network/threshold repo, we would always want to use CI workflow for file generation.


So I think, boiling down, the main decisions we need to take are:

  1. Where would we want to display the API docs?
    1A) Directly in the Threshold Docs Space under threshold-network/threshold/docs/app-development/api-docs/.
    1B) In the dedicated spaces, linked from threshold-network/threshold/docs/app-development/api-docs/?
    I think I prefer the solution 1B, because in those dedicated spaces we could decide on not providing the SUMMARY.md file, which (from what I read) would result in auto-generating the table of contents based on the synched repo structure. So we could configure Docgen to generate separate docs for each function or each Solidity file and GitBook should create a nice menu/TOC based on those files. The drawback is that in this solution we need to do more GitBook configuration than in the solution 1A (I hope that creating a space is quite a quick action).

  2. Where would we want to store the generated files? If we'll have dedicated spaces for each project API, we could:
    2A) store the .md file(s) in the original projects' repos and sync the folders where they're stored with the GitBook spaces
    2B) store the .md file(s) in designated new repos (one repo per project) and sync each repo with a separate GitBook space
    2C) store the .md file(s) in one new repo (for example threshold-network/threshold-api-docs), each project's API in a different folder and sync each folder with a separate GitBook space
    2D) store the .md file(s) in a designated part of the threshold-network/threshold repo (for example in a new ./docs-api folder), each project's API in a different subfolder and sync each subfolder with a separate GitBook space.
    2E) we could mix above approaches per project, depending on our needs/preferences.
    My personal preference would be solution 2D - we would have all the docs in one repo. Each above solution requires having to create one gitbook.yaml per each project, but in 2D those files would have to be created only in one repo (threshold-network/threshold), not in each project's repo. To have a repo synched with GitBook I think we also need to add a GitBook app in each repo - with 2D we need the app only for threshold-network/threshold (which I assume we already have). Someone could argue that pushing files to threshold-network/threshold repo (as proposed in 2D) may be harder than storing them in the original projects' repos, but the pushing may be done as a step in the same reusable CI workflow that's used to generate the files, which we (at least we at Keep) need for each project we want to document anyway. And even if for some project it will make more sense to have the files stored in the project's repo, we can do that and configure the GitBook space there (which would be 2E approach).

I'm interested to hear what are your preferences. I hope what I wrote above didn't make the matters more confusing 😄 If anything is unclear or I missed some point / made some incorrect assumptions, please let me know!

@michalinacienciala
Copy link
Contributor

@pdyraga, @derekpierre, would love to hear your opinion on points 1 and 2.
If I'll hear no objections, I'm going to assume that 1B + 2D is the option to go with :)

@derekpierre
Copy link
Member Author

derekpierre commented Mar 17, 2023

Apologies, this skipped my mind for the last few days. Thanks for laying out the different options.

IMHO the simplest set-up is 1B and 2A, i.e.:

  • Have dedicated GitBook spaces for repos docs which can be auto-synced to some docs folder in the respective application/component's github repository; this sync can be dictated by repos github branch eg main or other for latest release; I'm less sure if it works with github tags.
  • Intrusion into dev is minimal since all that is needed is a .gitbook.yaml file in the repos that points to the docs folder, which is very simple - Here's an example, https://github.com/threshold-network/threshold/blob/main/.gitbook.yaml
  • Each application/component's repos can have their own process (CI or otherwise) to rebuild the relevant docs whenever/however they desire (whether when a release occurs, or just part of their build process, or ...) and done in their own repos. When the relevant docs folder gets updated, the Gitbook space automatically syncs and the corresponding docs are subsequently automatically updated in the UI.
  • Having individual repositories automatically push built files into the threshold docs repository does not seem ideal in my mind (not that I can't be swayed, just not ideal to me). This is the 2D/2E approach. The synced dedicated space can be set up to prevent edits to API docs being made in Gitbook UI, so only allow changes from the specific application/component's github repos (one-way syncing). If instead these repos pushed files to the threshold docs github repos, they can be edited (inadvertently or otherwise), and therefore can possibly lead to conflicts/lost changes.

@pdyraga
Copy link
Member

pdyraga commented May 15, 2023

@michalinacienciala Is the work you are doing under keep-network/tbtc-v2#553 addressing threshold-network/solidity-contracts as well? If so, maybe we should close this PR?

@pdyraga
Copy link
Member

pdyraga commented Jun 28, 2023

Closing in favor of #138

@pdyraga pdyraga closed this Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants